Skip to content

Close stdin after writing to prevent LFS smudge hang#123

Merged
jorio merged 1 commit into
jorio:masterfrom
grub-basket:fix/lfs-smudge-stdin-hang
Jul 23, 2026
Merged

Close stdin after writing to prevent LFS smudge hang#123
jorio merged 1 commit into
jorio:masterfrom
grub-basket:fix/lfs-smudge-stdin-hang

Conversation

@grub-basket

Copy link
Copy Markdown
Contributor

Problem

RepoTask.flowStartProcess writes to the child process's stdin but never closes the write channel:

if stdin:
    process.write(stdin.encode("utf-8"))

DownloadLfsObjects pipes an LFS pointer into git lfs smudge, which reads stdin until EOF. With the write channel left open, smudge blocks forever waiting for EOF, the task never finishes, and the UI stays busy.

Repro

Open a diff of an LFS-tracked file whose object isn't cached locally → the smudge task hangs indefinitely.

Fix

Call process.closeWriteChannel() after writing stdin, so children that read to EOF terminate normally.


Prepared with Claude Fable 5 (Low effort mode).

flowStartProcess wrote to the child's stdin but never closed the write
channel. DownloadLfsObjects pipes an LFS pointer into 'git lfs smudge',
which reads stdin until EOF; without closeWriteChannel() the process
blocks forever and the task never finishes, leaving the UI busy.

Close the write channel after writing stdin.
@jorio
jorio merged commit ccb6deb into jorio:master Jul 23, 2026
7 checks passed
@jorio

jorio commented Jul 23, 2026

Copy link
Copy Markdown
Owner

My understanding is git lfs smudge proceeds as soon as it has completed reading a pointer off stdin, whether it reaches EOF or not. This is why testLfsDownloadMissingObjectsToCache in our test suite passes without hanging. (Let me know if I'm wrong here!)

I think your fix can still prevent smudge from hanging if we pipe a pointer that it fails to parse completely, e.g. if it's an exotic version of LFS that expects a different format than what we supply.

In this case, fortunately our UI stays responsive and allows killing the underlying process if it takes too long. But having smudge fail instantly is still better UX, so your fix makes sense. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants